fix(palette): restore composer remount for never-activated pet widgets by Yeachan-Heo · Pull Request #4605 · Yeachan-Heo/gajae-code

Yeachan-Heo · 2026-08-16T06:47:00Z

What

Fixes a live editor-lifecycle defect in selector and command-palette overlay transitions, with regression coverage that exercises disposal semantics instead of merely checking child replacement.

Container.clear() terminally disposes attached children. Before this change, selector opens and queued-message-selector opens called clear() while the reusable composer was attached; Editor.dispose() then removed the tab-width listener. Later restores re-mounted that disposed editor, so runtime tab-width changes stopped updating composer layout after the first overlay cycle.

The implementation uses the established TUI detach-then-readd contract:

detach the reusable editor before selector and queued-message-selector opens;
detach reusable plain/framed mounts before pet-aware and non-pet restores clear the shared container;
preserve 52dad458's successor-ownership guard in remountComposer() unchanged;
restore from the queued-message selector through the pet-aware restoreComposer() so an active pet keeps its PetFramedEditor mount and composer reserve after closing/deleting/moving through the queue selector.

Current-dev value and non-duplication

Current dev (648d13d72d0749c068eb9278e04f2a6ba0b07b78) still does not detach the composer before SelectorController.showSelector, queued-message-selector opens, or the relevant restore clears. This PR therefore retains a distinct production fix plus end-to-end lifecycle coverage.

Reconstruction (base moved dev `6696988` → `27afb73` → `2bd7b4a` → `ceb3134` → `08bef6c` → `648d13d`)

The three original commits (d162301d08, cce1441cbe, 097c3b4a1b, then eef82ef5dc at base 27afb732b3) were rebased onto fresh dev 648d13d72d (via 2bd7b4a48c/ceb31349c2/08bef6cf88) with git rebase --committer-date-is-author-date, preserving contributor authorship Yeachan Heo <yeachan.heo@gmail.com> and original author dates. The intervening dev commits touch none of this PR's source files; the source delta is unchanged from the accepted review scope.
Review blockers 4958123531 + 4958165282 were fixed forward in 3196de229c (details in the review replies).

Exact evidence

Head: 3196de229c3980ed8886458f52c52147cf60b991
Base: dev 648d13d72d0749c068eb9278e04f2a6ba0b07b78
Binary diff digest: sha256:d7376d7fd523a31784246c3f91a23ad56e6b2843752e746bd723acce66857633 from git diff --binary --full-index --no-ext-diff 648d13d72d...3196de229c
Prior/superseded runs (not current evidence): 32181321301 (Dev CI on 6e5670266b — every product job green; superseded only by this fix-forward commit), 32154701523 (6d12e769e6, all product green), 32099895353 (eef82ef5dc, all product green), cancelled 32099837032/32181054337, and contract-red 32099835720/32099895625/32181321780 (the contract runs failed solely on the by-design needs-human verdict gate). Exact-head CI on 3196de229c: Dev CI 32185025067 — every product job green, including the new share-command-temp-security shard and the expanded input-controller-keybindings suite carrying the real-disposal regression. The only red is PR contract bootstrap, which fails by design while the body records needs-human.
Local exact-head validation on 3196de229c: 264 focused tests / 0 fail across all 15 named suites (incl. the new real-Container/CustomEditor queued-message disposal regression and the /share + /debug viewer suites), plus share-command-temp-security, debug/log-viewer, mcp-quarantine-surface, credential-auto-import-flows, interactive-mode-editor-component, resume-session-reentrancy; bun run --cwd=packages/coding-agent check clean; git diff --check clean.

Review state

The prior CHANGES_REQUESTED reviews by @snowykr (4952815267 at b5929c80ec; 4958123531 and 4958165282 at eef82ef5dc) are addressed head-on in 3196de229c; signed replies are posted on both exact-head reviews. Owner: @Yeachan-Heo. The sole merge blocker is a fresh non-author exact-head review and matching merge-approved verdict after current-head CI is green.

GJC verdict

gajae.pr-review-verdict.v1 needs-human sha256:d7376d7fd523a31784246c3f91a23ad56e6b2843752e746bd723acce66857633 reviewer:human reviewer-id:Yeachan-Heo evidence:https://github.com/Yeachan-Heo/gajae-code/actions/runs/32185025067

[repo owner's gaebal-gajae (clawdbot) 🦞]

which only admits the current overlay owner or a widget whose epoch still matches. A widget that never activated (pet.mode "off" at startup) claims neither, so InteractiveMode.restoreComposer() -- which always routes through petWidget.remountComposer() once init() creates the widget -- silently no-opped. Palette cancel/close paths then left their CommandPaletteComponent mounted in editorContainer, leaking the modal (issue #4604, three command-palette-interactive-host failures). Composer-mount authority and overlay ownership are separate concerns: a never-activated widget still owns its host's composer mount and must remount the plain editor exactly like the no-pet fallback, while only disposal or a live successor widget revokes the mount. The active-owner case now also remounts the framed editor instead of relying on the epoch clause alone. Lore-id: 4604-palette-pet-remount Constraint: must not weaken #4591 successor-takeover isolation Tested: bun test command-palette-interactive-host.test.ts (8/8) Tested: bun test gajae-pet-widget.test.ts (62 pass incl. 2 new regressions) Tested: bun test packages/tui/test/gajae-pet.test.ts (19 pass) Not-tested: live iTerm2 pet drag interaction (requires real terminal) Confidence: high Scope-risk: narrow Reversibility: trivial

chatgpt-codex-connector · 2026-08-16T06:47:05Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Yeachan-Heo · 2026-08-16T06:55:46Z

Root cause

InteractiveMode.restoreComposer() routes through petWidget.remountComposer() whenever a pet widget exists — which is always after init(), even with pet.mode: "off". #4591 gated remountComposer() behind #canMutateSharedUi(), which admits only (a) the current overlay-emitter owner or (b) a previously-activated widget whose ownership epoch still matches. A widget that never activated (pet off at startup: #ownedOverlayEpoch === 0, never registered in petOverlayEmitterOwners) satisfies neither clause, so remountComposer() became a silent no-op and the palette's CommandPaletteComponent stayed mounted in editorContainer — the exact leak in the three failing tests (palette cancel path, active-command guard path, rejected-extension recovery path).

Pre-#4591, remountComposer() mounted unconditionally; for a never-activated widget #mountEditor(false) is byte-identical to the no-pet fallback (editorContainer.clear(); addChild(editor)). The fix separates composer-mount authority from overlay ownership: a never-activated widget still owns its host's composer mount and must remount the plain editor; only disposal or a live successor widget revokes the mount. #4591's successor-takeover isolation is untouched (still covered by the existing "ignores stale off and remount calls after successor takeover" test), and #canMutateSharedUi() gating in dispose()/#applyMode() is unchanged.

Validation (exact PR head `c194e79`)

Gate	Result
`bun test packages/coding-agent/test/command-palette-interactive-host.test.ts`	8 pass / 0 fail (was 3 fail at dev head `e1849e6`)
`bun test packages/coding-agent/test/gajae-pet-widget.test.ts`	62 pass / 0 fail (incl. 2 new regression tests)
`bun test packages/tui/test/gajae-pet.test.ts`	19 pass / 0 fail
`bun run --cwd packages/tui check`	clean (biome + tsc)
`bun run --cwd packages/coding-agent check`	clean
Adjacent suites (selector-controller-command-palette, qa-pet-restore-redteam, interactive-mode-editor-component, hook-editor, model-selector ×2)	104 pass / 0 fail

CI on this head (run 31932247745 + 31932247752)

Green: affected-path plan, native-build, test:packages/coding-agent/test/gajae-pet-widget.test.ts, ts-build (coding-agent), gjc-state-gates ×4, Local public surfaces, Virtual integration validation.

Red: PR contract bootstrap — the PR body lacks a gajae.pr-review-verdict.v1 merge-approved verdict line from an independent reviewer with an exact-head approval (governance gate; needs a maintainer review, not a code change). Bounded blocker report filed on #4604.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

Yeachan-Heo · 2026-08-17T16:11:36Z

Status: rebase complete, exact-head CI driven, human-review boundary reached.

Head: b5929c80eca5074998e3875c0b13a7a67076d9d1 (force-pushed with lease; old head c194e79453 was stale/DIRTY against new dev)
Base: dev 8711c555da (was e1849e676b; dev moved and contains 52dad458, the repo owner's independent fix for the same root cause)
Semantic conflict resolution: this PR's original remountComposer() implementation was dropped in favor of dev's shipped one (owner === undefined || owner === this) — byte-identical semantics for both protected properties (never-activated remount, successor ownership). The branch's surviving delta is exactly the two regression tests dev's fix shipped without (+55 lines, test file only).
Validation on exact head: command-palette-interactive-host 8/8, gajae-pet-widget 62 pass (incl. 2 new), tui gajae-pet 19 pass, --cwd packages/tui check and --cwd packages/coding-agent check clean.
CI on exact head: plan, gjc-state-gates ×4, Local public surfaces green; PR-contract checks now parse the verdict line correctly — the only red is intentional: needs-human blocks merge until an independent review arrives.
Review request: @snowykr (documented maintainer, dev 7ca1d66a60) — please review head b5929c80ec and approve. After approval the verdict line in the PR body must be flipped to merge-approved with digest 11193ebfd7e38991253bb7bb859fff515ae7caa34af61897a83ff9a5a8794505 (exact 8711c555da...b5929c80ec diff) and reviewer-id:snowykr; I will then squash-merge and reconcile the issue.

No code or behavior delta vs dev; this PR only adds the regression coverage that would have caught #4591's leak before it shipped.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

Yeachan-Heo · 2026-08-17T16:24:40Z

Blocked solely on human review — full state for the reviewer

Head: b5929c80eca5074998e3875c0b13a7a67076d9d1 · Base: dev 8711c555da490ad36475f94368bcdd8d8f01048e · Owner branch: fix/issue-4604-command-palette-pet-restore (force-pushed with lease)
Diff vs dev: one file, packages/coding-agent/test/gajae-pet-widget.test.ts +55 lines — two regression tests. Zero source delta: the root-cause fix itself is already on dev as 52dad458; this PR only adds the coverage it shipped without.
Tests pin: (1) never-activated widget + foreign overlay in editorContainer → remountComposer() restores [editor] (the exact fix(dev): command-palette-interactive-host failures from #4591 pet integration #4604 leak); (2) active widget → restores [framedEditor]. Successor-ownership protection remains pinned by the pre-existing takeover test.
Local reproduction: on dev 8711c555da before 52dad458, command-palette-interactive-host was 3-fail; with the fix plus this coverage it is 8/8, gajae-pet-widget 62 pass, tui gajae-pet 19 pass, tui + coding-agent check clean.
CI on exact head (run 32044402879): all code jobs green — plan, native-build, test:gajae-pet-widget, ts-build, evidence producer, Affected path validation, Virtual integration validation, gjc-state-gates ×4. (Run 32044088151 was concurrency-cancelled mid-flight when the body edit superseded it; its "native build" failure is the cancellation, not a code failure.)
Blocker: PR contract bootstrap / Validate exact-head PR contract intentionally block on the recorded needs-human verdict until an independent non-author review exists. Reviewer must hold admin/maintain/write. @snowykr is requested (documented maintainer, dev 7ca1d66a60).
After approval: flip the verdict line in the PR body to
gajae.pr-review-verdict.v1 merge-approved sha256:11193ebfd7e38991253bb7bb859fff515ae7caa34af61897a83ff9a5a8794505 reviewer:human reviewer-id:<reviewer-login> evidence:<review url>, rerun PR contract, then squash-merge to dev. Issue fix(dev): command-palette-interactive-host failures from #4591 pet integration #4604 is already closed; no further issue reconciliation needed.
Terminal disposition: hold at this boundary. No speculative merge, no self-approval. The PR is a pure test-addition on top of an already-shipped fix — lowest-risk possible merge for dev.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

snowykr

Verdict: Request changes

gajae.pr-review-verdict.v1 request-changes sha256:1465ccdfd915bdfdc6b32a6431efa134faeecd6346a209b4ab49cd99aee45faf reviewer:human reviewer-id:snowykr evidence:https://github.com/Yeachan-Heo/gajae-code/actions/runs/32044402879

Reviewed the exact PR head b5929c80eca5074998e3875c0b13a7a67076d9d1 against dev 8711c555da across intent/contract, architecture/correctness, security/trust, verification/CI, and compatibility/platform context.

The regression-test direction is correct, and I found no actionable P1, P3, or P5 issue. The two new assertions cover the ownerless/off → plain-editor and active-owner → framed-editor remount branches, while the existing successor-takeover test preserves the stale-owner contract.

Required changes

1. Make the remount tests model the reusable editor lifecycle

Location: packages/coding-agent/test/gajae-pet-widget.test.ts:650-701

makeStubs() supplies an editor cast to CustomEditor, but the stub has no dispose() implementation. The production #mountEditor() uses editorContainer.clear(), and Container.clear() disposes every current child. CustomEditor.dispose() is not a no-op: it tears down the tab-width listener and resets pending paste state. The new tests therefore prove only child replacement; they can pass even if a repeated palette close/remount path has disposed the reusable editor.

Please either use a real/disposable editor in this regression or add a focused lifecycle assertion that repeats the restore path and verifies the editor remains usable (including the relevant disposal/listener contract). If the intended ownership model is detach-then-readd, the implementation/test should make that explicit rather than relying on a stub that omits disposal.

2. Refresh the exact-head evidence in the PR contract

The PR body’s verdict still points to https://github.com/Yeachan-Heo/gajae-code/actions/runs/32044088151, but that is the older/cancelled run and gh pr checks 4605 still reports its affected-path job as failed. The successful exact-head product run is 32044402879; its targeted gajae-pet-widget test, ts-build, native-build, affected-path plan/evidence, and integration jobs pass, while the contract jobs fail because the body still declares needs-human.

Please update the verdict evidence to the immutable successful exact-head run (or explicitly label the old URL historical), then rerun the exact-head contract gate after the review decision is recorded.

Review lane summary

P1 Intent / Policy / Contract: clear.
P2 Architecture / Correctness / Failure: changes requested for lifecycle/test fidelity; active-pet InteractiveMode integration remains a useful follow-up gap.
P3 Security / Privacy / Trust: not materially applicable; test-only change with no external or security boundary.
P4 Verification / Tests / CI: targeted changed-test CI is green, but neighboring host/TUI suites are local-only claims and the body evidence URL is stale.
P5 Context / Compatibility / Platform: clear; no generated artifacts, runtime/package changes, or changelog required for this test-only diff.

Yeachan-Heo · 2026-08-17T17:26:21Z

Exact-head CHANGES_REQUESTED at b5929c80eca5074998e3875c0b13a7a67076d9d1 is assigned to the live fix-forward lane.

Owned blockers:

Replace the child-replacement-only editor stub coverage with a real/disposable repeated restore/remount lifecycle proof, or fix production to detach/readd safely, so Container.clear() cannot silently dispose the reusable CustomEditor and its listener/paste state.
Replace the stale verdict evidence URL for cancelled run 32044088151; current successful product evidence is 32044402879, and a code push will require a new exact-head run again.

The lane will implement, push, rerun exact-head CI, obtain a fresh approval/verdict, and drive the PR to dev merge. This is not a human-only hold.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

…store Container.clear() disposes children terminally, and Editor.dispose() tears down the tab-width change listener. Opening a selector cleared editorContainer with the live composer attached, so the first palette round-trip silently killed the editor's listener; every later restore re-mounted a dead editor and runtime tab-width changes stopped re-deriving composer layout. Overlay open paths now detach the reusable editor first (Container.detachChild, the tui detach-then-readd reuse contract), and the pet-aware restores (#mountEditor, restoreComposer, queued-message restore) detach both reusable mounts before clearing. Successor ownership isolation in remountComposer is unchanged. Regression coverage: a real-CustomEditor lifecycle test in the widget suite (disposal observably stops tab-width invalidations) and an end-to-end host test that fails on the pre-fix open path and passes with the fix (verified red/green by reverting the source change). Lore-id: pr4605-composer-lifecycle Constraint: preserve #4591 successor-takeover isolation (remountComposer untouched) Tested: red/green on command-palette-interactive-host (8+1 pass with fix; new test fails without) Tested: 264 pass across 12 adjacent suites; gajae-pet-widget 63 pass Tested: tui + coding-agent check clean; coding-agent build clean Confidence: high Scope-risk: moderate Reversibility: trivial

Yeachan-Heo · 2026-08-17T19:14:56Z

Requested changes addressed — head a890974ebb ready for re-review

Both required changes from review 4952815267 are implemented, tested red/green, and pushed to the authorized branch (lease-guarded force push from b5929c80ec).

1. Remount tests now model the reusable editor lifecycle. The stub editor was replaced with a real CustomEditor in a real Container for all three lifecycle tests. The core probe: Editor.dispose() tears down the tab-width change listener, and that listener fires editor.invalidate() on a runtime tab-width change — so counting invalidations across overlay cycles proves disposal never happened:

New widget test: 4 production-shaped open (detach → clear → overlay) / close (remountComposer) cycles; every cycle's tab-width toggle reaches the live listener (8 invalidations); a red control at the end proves the probe (a genuinely disposed editor accrues zero further invalidations).
New end-to-end host test (command-palette-interactive-host): repeated palette open/close through the real editor key path; each cycle's tab-width toggles still reach the live listener and the restored composer accepts input.
Red/green proof: reverting only the source fix makes the host test fail (8 pass / 1 fail); with the fix, 9 pass / 0 fail.

The review also exposed a real production defect, now fixed. showSelector opened overlays with editorContainer.clear() while the live composer was attached — and Container.clear() disposes children terminally. The first palette round-trip silently killed the editor's tab-width listener; every restore re-mounted a dead editor. Fix: overlay open paths detach the reusable editor first (Container.detachChild, the tui detach-then-readd reuse contract — the same pattern the hook selectors already use), and pet-aware restores detach both reusable mounts before clearing. Successor-ownership isolation is untouched (remountComposer is byte-identical with dev's 52dad458; the takeover test still pins it).

2. Verdict evidence refreshed to the immutable exact-head run. Exact-head Dev CI run 32058210086: 17 code jobs green — native-build, test:gajae-pet-widget, ts-build, affected-path plan + evidence producer, Virtual integration validation, gjc-state-gates ×4, Local public surfaces. The only red is the PR contract gate failing intentionally on the recorded needs-human verdict (blocked-report contract: no self-approval). Digest d97e9e79…cf2327 matches the exact 8711c555da...a890974ebb diff.

Validation on a890974ebb: command-palette-interactive-host 9/9, gajae-pet-widget 63/63, tui gajae-pet 19/19, 264 pass across 12 adjacent suites, tui + coding-agent check clean, coding-agent build clean.

Follow-up gap (separate issue): share/gist loader, MCP wizard, and tasks-pane paths still use the raw clear() pattern.

@snowykr — ready for a fresh review of a890974ebb.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

Yeachan-Heo · 2026-08-17T19:19:01Z

Terminal disposition: blocked solely on fresh human review of a890974ebb

Exact head: a890974ebb46d9dcb4c75beb5f6f166e3854c2df · Base: dev 8711c555da · Branch: fix/issue-4604-command-palette-pet-restore (lease-guarded force push; no further mutations pending)
CI on exact head: Dev CI run 32058210086 — 17/17 code jobs green (native-build, test:gajae-pet-widget, ts-build, affected-path plan + evidence producer, Virtual integration validation, gjc-state-gates ×4, Local public surfaces). Sole red is the PR contract gate failing by design on the recorded needs-human verdict. Evidence URL in the PR body now points at this exact-head run (stale 32044088151 reference replaced).
Review state: snowykr's CHANGES_REQUESTED (4952815267) applied to the previous head b5929c80ec. Both required changes are implemented on a890974ebb: (1) real-editor lifecycle regression with red/green proof — reverting only the source fix fails the new host test, with it 9/9; the review additionally exposed a production defect (overlay open disposed the live composer via Container.clear()), fixed via the tui detach-then-readd contract; (2) body evidence refreshed to the immutable exact-head run. Fresh re-review requested from snowykr.
Local reproduction (dev base): widget suite 63/63; palette host suite 9/9; 264 pass across 12 adjacent suites; tui pet 19/19; --cwd packages/tui + --cwd packages/coding-agent checks clean; coding-agent build clean.
Owner: gaebal-gajae (clawdbot) lane under Yeachan-Heo auth; blocked-report contract forbids self-approval, so the merge-approved verdict cannot be issued from this side.
Blocker: one authenticated APPROVED review of a890974ebb by a non-author maintainer (snowykr re-requested). On approval: flip the body verdict to merge-approved sha256:d97e9e7944dbde6af6530a4c989097d6d01eef4352b707c8b9e554f1d3cf2327 reviewer:human reviewer-id:<approver> evidence:<run 32058210086>, rerun the PR-contract check, then squash-merge to dev and reconcile issue fix(dev): command-palette-interactive-host failures from #4591 pet integration #4604 (already closed; no further action needed there).

Lane holds here; no speculative merge.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

Yeachan-Heo · 2026-08-17T19:36:02Z

Terminal disposition: blocked solely on fresh human review of cce1441cbe (supersedes the same disposition at a890974ebb after the dev rebase)

Exact head: cce1441cbe23d941e90b74b6c65ed42064cfed97 · Base: dev 6696988b6dd757f898ab7b33984ba830b6344109 · Branch: fix/issue-4604-command-palette-pet-restore (rebased from a890974ebb after fix(omlx): keep role thinking effort on discovered models #4631/fix(utils): recognize exported dotenv assignments #4613 landed; only conflict was the CHANGELOG Unreleased section, resolved by keeping both entries — no merged behavior and no fix(palette): restore composer remount for never-activated pet widgets #4605 fix content dropped; git diff --check clean)
Exact-head CI (run 32060135954, final): 19 product jobs green — affected-path plan, native-build, test:gajae-pet-widget, test:command-palette-interactive-host, test:selector-controller-command-palette, test:input-controller-keybindings, test:model-selector-* ×2, check:@gajae-code/coding-agent, ts-build, cli-smoke, evidence producer, Virtual integration validation, gjc-state-gates ×4, Local public surfaces. Sole red: PR contract bootstrap, failing by design on the recorded needs-human verdict (Verdict needs-human intentionally blocks merge). No product failure to classify.
Local verification on the rebased head: command-palette-interactive-host + gajae-pet-widget 72/72; 197 pass across 11 adjacent suites (incl. omlx-discovery from the newly merged fix(omlx): keep role thinking effort on discovered models #4631); tui gajae-pet 19/19; --cwd packages/tui and --cwd packages/coding-agent checks clean; git diff --check clean.
Review state: snowykr's CHANGES_REQUESTED (4952815267) still targets the pre-rebase head; both requested changes remain implemented. Fresh re-review of cce1441cbe requested; PR body evidence and digest (46049afc…06c4a, exact 6696988b6d...cce1441cbe diff) refreshed to this head.
Blocker (sole): one authenticated APPROVED review of cce1441cbe by a non-author maintainer. On approval: flip the body verdict to merge-approved sha256:46049afcda5f9b6f2c53b1e41054d1e8b54cad93191d28a24afe940778f06c4a reviewer:human reviewer-id:<approver> evidence:<run 32060135954>, rerun the PR-contract check, squash-merge to dev, reconcile fix(dev): command-palette-interactive-host failures from #4591 pet integration #4604 (already closed).

No mutations while review is absent: no self-approval, no verdict change, no source/body edits. Lane returns to external review hold.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

The framed-remount regression test constructs a real editor to exercise container disposal semantics, but does not otherwise need the local binding. Removing it keeps the affected-package check warning-free. Lore-id: pr4605-lint-cleanup Tested: bun test packages/coding-agent/test/gajae-pet-widget.test.ts (63 pass) Tested: bun run --cwd=packages/coding-agent check Confidence: high Scope-risk: trivial Reversibility: trivial

Yeachan-Heo · 2026-08-17T21:57:59Z

@snowykr Fresh non-author review requested for exact head 097c3b4a1b4c30910bb00548dde89b74877a936a against dev 6696988b6dd757f898ab7b33984ba830b6344109.

The prior CHANGES_REQUESTED is stale (b5929c80ec) and is not counted. Its lifecycle-proof finding is fixed forward: the real-CustomEditor regression now proves repeated overlay transitions preserve the tab-width listener and editor usability. Local exact-head evidence is 63 pass / 0 fail for gajae-pet-widget, clean coding-agent check, and clean diff check. Binary diff digest: sha256:011a3260f7310924e4956d92da7ce6cb059f149339ac7d98fe86d6656113df34.

Current exact-head CI is running. The only expected governance blocker after product CI is a fresh independent approval and matching merge-approved verdict.

[repo owner's gaebal-gajae (clawdbot) 🦞]

Yeachan-Heo · 2026-08-18T04:39:01Z

Reconstruction evidence — PR rebased onto current dev, fresh exact-head review requested

Old base → new base: dev 6696988b6dd757f898ab7b33984ba830b6344109 → 27afb732b3d25632d44176687d5bdd78d3419bb3 (14 commits; none touch this PR's source files).
Old head → new head: 097c3b4a1b4c30910bb00548dde89b74877a936a → eef82ef5dc5a620f847312057d3ea1eab7bbe55d, pushed with --force-with-lease to the authorized branch fix/issue-4604-command-palette-pet-restore (lease pinned to the old head).
Replay method: git rebase --committer-date-is-author-date 27afb732b3 097c3b4a1b — all three accepted commits (d162301d08, cce1441cbe, 097c3b4a1b) rebased cleanly; contributor authorship Yeachan Heo <yeachan.heo@gmail.com> and original author dates preserved; only the CHANGELOG.md Unreleased hunk context moved. Source+test delta vs the new base is byte-identical to the accepted PR delta vs the old base (verified by diff comparison); zero unrelated drift entered the branch.
Canonical digest: sha256:59e7bd2abe687245725790324531d00da64e9f77f6e83f31ad47bb6e07680713 = sha256(git diff --binary --full-index --no-ext-diff 27afb732b3d25632d44176687d5bdd78d3419bb3 eef82ef5dc5a620f847312057d3ea1eab7bbe55d).
Local exact-head validation on eef82ef5dc: focused suites 172 pass / 0 fail across gajae-pet-widget (63, incl. the three lifecycle tests), command-palette-interactive-host, selector-controller-command-palette, input-controller-keybindings, model-selector-controller-batch, model-selector-profiles; bun run --cwd=packages/coding-agent check clean; bun run --cwd=packages/coding-agent build exit 0; git diff --check clean; check:schemas + check:plugins in sync.
Exact-head CI: Dev CI run 32099837032 and PR-contract run 32099835720 are running on eef82ef5dc. The PR-contract needs-human failure is expected and by design until the fresh review lands.
Verdict line: the PR body carries exactly one current gajae.pr-review-verdict.v1 needs-human line bound to the digest above.

Review request: @snowykr — fresh authorized non-author review of exact head eef82ef5dc5a620f847312057d3ea1eab7bbe55d against dev 27afb732b3d25632d44176687d5bdd78d3419bb3, please. The prior CHANGES_REQUESTED (review 4952815267) targeted b5929c80ec; its lifecycle-proof request was fixed forward in cce1441cbe with real-CustomEditor disposal-probe tests, so it is stale. After product CI is green, a fresh merge-approved verdict bound to sha256:59e7bd2abe687245725790324531d00da64e9f77f6e83f31ad47bb6e07680713 unblocks merge.

Signed: owner lane for PR #4605, head eef82ef5dc5a620f847312057d3ea1eab7bbe55d, base 27afb732b3d25632d44176687d5bdd78d3419bb3, digest sha256:59e7bd2abe687245725790324531d00da64e9f77f6e83f31ad47bb6e07680713, pushed 2026-08-18 via force-with-lease.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

Yeachan-Heo · 2026-08-18T05:57:58Z

Owner-lane boundary review complete — head eef82ef5dc stands as-is; review remains the sole blocker

The owner lane has finished its adversarial boundary review of exact head eef82ef5dc5a620f847312057d3ea1eab7bbe55d vs dev 27afb732b3d25632d44176687d5bdd78d3419bb3 (digest sha256:59e7bd2abe687245725790324531d00da64e9f77f6e83f31ad47bb6e07680713):

All 5 changed sites verified correct: detach-then-readd is the documented Container contract; every restore pairs re-add with setFocus; drafts, paste state, and the tab-width listener provably survive overlay round-trips; no double-mount/double-dispose; the framed-editor hypothesis was investigated and disproven (PetFramedEditor has no dispose, clear() calls child.dispose?.()).
Regression tests verified non-vacuous by independent red-team runs: the 4-cycle probe asserts exactly 8 invalidations with an explicit red control proving the counter detects the old bug; the host test drives a real InteractiveMode through the editor's real key handler; full focused surface 172 pass / 0 fail (JUnit: 172 tests, 0 failures, 0 errors).
Exact-head CI (run 32099895353): every product job green (ts-build, cli-smoke, native-build, all 5 affected test suites, affected-path plan + evidence, Virtual integration, gjc-state-gates ×4). The only red job is PR contract bootstrap, failing exactly as designed: Verdict needs-human intentionally blocks merge.

Advisory findings (not merge blockers, recorded for follow-up): the same terminal-disposal defect remains reachable at four pre-existing unguarded open paths — jobs overlay (alt+j, /monitors — reachable from this PR's own palette), tasks pane (alt+t), OAuth API-key paste login (~20 providers), and /debug viewers. None is a regression from this PR; all predate it on the base commit. They belong in a separate issue/PR, not scope-crept into this one.

@snowykr — the review request from the reconstruction comment still stands: fresh non-author review of exact head eef82ef5dc, base 27afb732b3. The prior CHANGES_REQUESTED (review 4952815267) targeted b5929c80ec and its lifecycle-proof request was fixed forward in cce1441cbe with real-CustomEditor disposal-probe tests, so it is stale. A fresh merge-approved verdict bound to sha256:59e7bd2abe687245725790324531d00da64e9f77f6e83f31ad47bb6e07680713 unblocks merge; the owner lane will then merge to dev, dogfood fresh dev with bun run build, and reconcile post-merge CI.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

snowykr

Verdict: Request changes

gajae.pr-review-verdict.v1 request-changes sha256:59e7bd2abe687245725790324531d00da64e9f77f6e83f31ad47bb6e07680713 reviewer:human reviewer-id:snowykr evidence:https://github.com/Yeachan-Heo/gajae-code/actions/runs/32099895353

Reviewed the new exact head eef82ef5dc5a620f847312057d3ea1eab7bbe55d against dev 27afb732b3d25632d44176687d5bdd78d3419bb3 across the requested P1–P5 axes, including the detach-before-clear production changes and the new lifecycle tests.

The production direction is sound: detaching the reusable editor before Container.clear() preserves the editor's terminally-owned lifecycle, and the exact-head affected-path jobs for the changed suites are green. I found one blocking verification/compatibility issue before approval.

Required change

Update all SelectorController test contexts for the new `detachChild` requirement

Locations:

packages/coding-agent/test/custom-model-preset-creation.test.ts:854-858
packages/coding-agent/test/model-selector-profiles-redteam.test.ts:127-127,356-356
packages/coding-agent/test/provider-onboarding-wizard.test.ts:263-269
packages/coding-agent/test/provider-onboarding-wizard-redteam.test.ts:320-327

These contexts instantiate SelectorController and exercise selector-opening paths such as showModelSelector() / showCustomProviderWizard(), which all enter SelectorController.showSelector(). The new implementation now unconditionally calls this.ctx.editorContainer.detachChild(this.ctx.editor) before clear(), but these test doubles still provide only clear() and addChild(). Because they are cast to InteractiveModeContext/never, TypeScript does not catch the missing method; the affected tests will fail at runtime with detachChild is not a function when the selector opens.

Please update every lightweight editorContainer double that reaches SelectorController.showSelector() to implement detachChild with the same child-list semantics as the existing updated fixtures. A repository-wide audit should include the session-selector and login-selector test contexts as well, rather than updating only the changed-path suites.

Verification notes

P1 Intent / Contract: clear; the detach-then-readd change matches the stated lifecycle contract.
P2 Architecture / Correctness: no production blocker found in the reviewed call sites; PetFramedEditor is not disposable, and the reusable CustomEditor remains owned by InteractiveMode.
P3 Security / Privacy / Trust: not materially applicable; no new external or sensitive-data boundary.
P4 Verification / CI: the new lifecycle tests are meaningful, but current CI targets only the changed suites and therefore does not catch the unupdated selector test doubles above.
P5 Compatibility / Platform: runtime API usage is consistent with Container; test fixtures need to be brought to the same contract. The changelog entry is appropriate for the user-visible lifecycle fix.

The current exact-head product run is 32099895353; its relevant affected-path jobs pass. The contract jobs remain red by design while the PR body records needs-human.

snowykr

Verdict: Request changes

gajae.pr-review-verdict.v1 request-changes sha256:59e7bd2abe687245725790324531d00da64e9f77f6e83f31ad47bb6e07680713 reviewer:human reviewer-id:snowykr evidence:https://github.com/Yeachan-Heo/gajae-code/actions/runs/32099895353

Follow-up review of the same exact head eef82ef5dc5a620f847312057d3ea1eab7bbe55d found two additional blocking gaps in the fix-forward. The detach lifecycle direction is correct, but the queued-message path and the test/evidence contract are not complete yet.

Required changes

1. Restore the pet-aware composer from the queued-message selector

Location: packages/coding-agent/src/modes/controllers/input-controller.ts:1249-1255

#restoreEditorFocus() now detaches the raw editor and re-adds that raw editor directly. In an active pet session, the mounted child is PetFramedEditor, not ctx.editor; the detach is therefore a no-op, clear() removes the framed wrapper, and the close path mounts the bare editor. The pet remains active and continues emitting its sprite, but the composer reserve/frame is gone after closing or deleting/moving through the queued-message selector.

This is deterministic for the active-pet queue flow and is exactly the kind of reusable-composer contract this PR is fixing. Route the restore through ctx.restoreComposer() (or an equivalent pet-aware mount helper) so the active mode restores PetFramedEditor and off mode restores the plain editor. Add an active-pet queued-selector regression that asserts the framed child and reserve survive open/close.

2. Update every selector test double that now needs `detachChild`

The first follow-up only updated the changed-path fixtures. Several existing contexts still instantiate SelectorController and enter showSelector(), but provide only clear() and addChild(). Since showSelector() now calls detachChild() unconditionally, these tests can fail at runtime despite being cast through InteractiveModeContext/never:

packages/coding-agent/test/custom-model-preset-creation.test.ts:854-858
packages/coding-agent/test/model-selector-profiles-redteam.test.ts:127,356
packages/coding-agent/test/provider-onboarding-wizard.test.ts:263-269
packages/coding-agent/test/provider-onboarding-wizard-redteam.test.ts:320-327
packages/coding-agent/test/modes/components/theme-selector-input.test.ts selector contexts
packages/coding-agent/test/modes/components/thinking-selector.test.ts selector contexts
packages/coding-agent/test/selector-controller-resume-model.test.ts:11-15
packages/coding-agent/test/selector-controller-session-delete.test.ts:11-15 and 49-57

Please audit all SelectorController fixtures that reach showSelector() and add a child-list-correct detachChild implementation, not just the affected-path files. The current targeted CI does not execute all of these suites.

3. Refresh the exact-head evidence references

The PR body still identifies 32099837032 / 32099835720 as the current exact-head evidence. gh reports 32099837032 cancelled and 32099835720 failed. The completed current-head product run is 32099895353; its product/affected jobs pass, while 32099895625 is the separate contract run that fails only because the body still records needs-human.

Update the body’s exact-evidence section and verdict URL to the completed run, and describe the contract failure as the expected human-review gate. Do not leave cancelled/failed runs presented as current evidence.

Additional verification note

The new real-editor widget lifecycle tests at packages/coding-agent/test/gajae-pet-widget.test.ts:671-702 and the preceding active/off tests should explicitly dispose the CustomEditor fixture after each test. widget.dispose() does not own the editor, and CustomEditor registers a process-wide tab-width listener; the first two real-editor tests can leak that listener into later tests. The repeated lifecycle test’s red control disposes it intentionally, but the other real-editor fixtures need equivalent cleanup.

The existing raw editorContainer.clear() swaps outside SelectorController.showSelector() (OAuth code input, jobs/tasks panes, command-controller loaders, and runtime-MCP wizard) also deserve a lifecycle audit before claiming the reusable composer contract is complete; I am treating those as follow-up scope unless they are intended to be covered by this PR’s broad selector/overlay fix.

Lane summary

P1 Intent / Contract: changes requested for queued pet-aware restore and incomplete fixture contract.
P2 Architecture / Correctness: active-pet queued selector loses PetFramedEditor deterministically.
P3 Security / Privacy / Trust: no actionable finding; not materially applicable.
P4 Verification / CI: lifecycle coverage is improved, but unupdated fixtures and stale evidence remain.
P5 Compatibility / Platform: runtime Container usage is consistent; all lightweight contexts must match the new method contract.

…ctor Review 4958165282 on the exact head found the queued-message close path still swapped in the raw editor: in an active pet session the mounted composer child is PetFramedEditor, so #restoreEditorFocus() detached the plain editor (a no-op), let clear() drop the framed wrapper, and mounted the bare editor — the pet kept emitting while the composer lost its reserve. Route the restore through ctx.restoreComposer() (the same pet-aware helper SelectorController.showSelector's done() already prefers), falling back to the plain editor swap for contexts that predate it. Also bring every remaining SelectorController test double to the new detachChild contract (child-list-correct, not a no-op cast) and dispose the real CustomEditor fixtures in the pet-widget lifecycle tests so their process-wide tab-width listeners cannot leak into later tests. Lore-id: pr4605-review-4958165282 Constraint: preserve contributor authorship and one-PR scope Tested: bun test input-controller-keybindings (65), gajae-pet-widget, command-palette-interactive-host, selector-controller-command-palette, custom-model-preset-creation, model-selector-profiles-redteam, provider-onboarding-wizard(+redteam), theme-selector-input, thinking-selector, selector-controller-resume-model, selector-controller-session-delete, login-preset-recommendation, model-selector-controller-batch, model-selector-profiles (all green) Tested: bun run --cwd=packages/coding-agent check Confidence: high Scope-risk: moderate Reversibility: trivial

…y opens Issue #4657. Four overlay-open paths still cleared editorContainer with the live composer attached: the jobs overlay (alt+j //monitors), the tasks pane (alt+t), the OAuth API-key paste input, and the /debug log + raw-SSE viewers. Container.clear() disposes children terminally and Editor.dispose() tears down the tab-width change listener, so the first round-trip on any of these paths silently killed the composer's listener; every later restore re-mounted a dead editor and runtime tab-width changes stopped re-deriving composer layout. Each open now detaches the reusable editor first (Container.detachChild, the same detach-then-readd reuse contract showSelector and the extension-ui and pet-aware restore paths already follow from PR #4605), so the terminal clear disposes only the transient overlay. User-facing behavior is unchanged: the overlay mounts exactly as before and the composer is re-added on close. Regression coverage: one deterministic test per named path in composer-detach-overlay-paths.test.ts driving the real production open/close code (real JobsOverlayComponent/TasksPaneComponent focus lists, the captured production onPrompt closure over a real Input, and the real DebugSelector selection into the real DebugLogViewerComponent with an isolated agent-dir log source), each asserting the tab-width listener still fires across four overlay cycles, with a red control proving the probe detects genuine disposal. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes untouched (showSelector/extension-ui/pet restore shapes unchanged) Tested: red/green — 4/4 fail on unfixed tree, 4/4 pass with fix Tested: 107 pass across 9 adjacent suites (pet widget, editor dispose, tasks, jobs, debug, login) Tested: 190 pass across 7 selector/palette/input suites Tested: bun run --cwd=packages/coding-agent check + build clean; ci:test:smoke ok Tested: check:tools, node20-baseline, public-sync, schemas, docker-context, gjc-ui gates pass Confidence: high Scope-risk: narrow Reversibility: trivial

Yeachan-Heo · 2026-08-18T15:34:14Z

Addressed in full — fixture audit complete (review 4958123531)

Every SelectorController test context that reaches showSelector() now implements detachChild with child-list-correct semantics; the audit went beyond the four files you flagged to also cover theme-selector-input, thinking-selector, selector-controller-resume-model, selector-controller-session-delete, and the login-selector context in login-preset-recommendation. Contexts already using real Containers (credential-auto-import-flows, interactive-mode-*) or never reaching showSelector() (g007-ws5-redteam, sessions-dashboard, transcript-viewer-overlay, resume-progress-lease, resume-session-reentrancy*) needed no change. The follow-up review 4958165282 additionally found and fixed the queued-message pet-aware restore gap and the real-editor fixture disposal leak; both are resolved in the same head. All named suites re-run green locally. Details in the replies on review 4958165282.

New exact head for re-review: 6d12e769e65f174a4615b87da9aea5d9b1b1318d against dev ceb31349c2d024c75825c819cb530d5060f49380, digest sha256:4221be9bfe1f9ffad63811decb0be31cc43369d59e3ad8b607522d28dee52eba, exact-head CI run 32154050225.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Addressed — pet-aware queued-message restore + regression coverage (required change 1 of review 4958165282)

Fixed in 6d12e769e65f174a4615b87da9aea5d9b1b1318d (11f772ac64 before the dev rebase): InputController.#restoreEditorFocus() now routes through ctx.restoreComposer() — the same pet-aware helper SelectorController.showSelector's done() already prefers — and only falls back to the plain detach/clear/re-add swap for contexts that predate it. In an active pet session the restore now re-mounts PetFramedEditor (preserving the composer reserve); in off mode it re-mounts the plain editor, exactly as before.

Deterministic regressions added in packages/coding-agent/test/input-controller-keybindings.test.ts:

Close (⎋): with restoreComposer present, cancelling the queue selector calls it exactly once and the container ends with the composer, never the raw-editor swap — pins the active-pet framed-mount contract (the mock stands in for InteractiveMode.restoreComposer, which remounts the framed wrapper when the pet is active).
Delete-to-empty (⌦ ⌦): deleting down to an empty queue closes through the pet-aware restore; queue ends empty.
Move + delete (⌃↓ ⌦ ⌦): reorder through the selector, then delete to empty — still the pet-aware restore.
Fallback: when restoreComposer is absent (off mode / lightweight doubles), the close still restores the plain editor with setFocus(editor).

All 65 tests in that suite green locally.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Addressed — repository-wide SelectorController fixture audit + lifecycle fixture disposal (required changes 2 and 3 of review 4958165282)

Fixture audit. Every test context that instantiates SelectorController and reaches showSelector() was audited repository-wide (new SelectorController\( across packages/coding-agent/test). Doubles with only clear()/addChild() were brought to the detachChild contract with child-list-correct semantics (splice from the tracked children array — not a no-op), matching the already-updated fixtures:

custom-model-preset-creation.test.ts (double without a child list; no-op detach is correct there since addChild only captures selector instances)
model-selector-profiles-redteam.test.ts (both contexts, lines 127 and 356)
provider-onboarding-wizard.test.ts + provider-onboarding-wizard-redteam.test.ts (child-list-correct splice)
theme-selector-input.test.ts (both contexts)
thinking-selector.test.ts (all four contexts)
selector-controller-resume-model.test.ts
selector-controller-session-delete.test.ts
login-preset-recommendation.test.ts (the login-selector context, covering the "session/login selectors" ask)

Verified non-fixtures: credential-auto-import-flows and interactive-mode-editor-component / interactive-mode-background-activity already use real Containers (real detachChild); g007-ws5-redteam, sessions-dashboard, transcript-viewer-overlay, resume-progress-lease, and both resume-session-reentrancy suites never reach showSelector(). All named suites re-run green locally (87 pass across the nine updated files; 140 across pet/palette/queue suites).

Fixture disposal. The three real-CustomEditor pet-widget lifecycle tests now dispose their editors in finally (widget.dispose(); editor.dispose()), and the repeated-cycle test's red control is followed by an explicit idempotent editor.dispose() so the process-wide tab-width listener can never leak into later tests even if the red-control clear were removed.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Addressed — evidence refresh + adjacent-path audit (required changes 4 of 4958165282, and the audit note)

Evidence. The PR body's exact-evidence section now names head 6d12e769e6, base ceb31349c2, digest sha256:4221be9bfe1f9ffad63811decb0be31cc43369d59e3ad8b607522d28dee52eba, and cites only the fresh exact-head run 32154050225 (Dev CI) as current evidence. The old 32099837032 (cancelled) / 32099835720 / 32099895625 references are retained only as superseded history with their states labeled truthfully (contract runs failed solely on the by-design needs-human gate); 32099895353 is explicitly marked as the prior head's completed product run.

Adjacent raw clear() swaps. Audited against the contract this PR introduces. The OAuth code input, jobs/tasks panes, command-controller loaders, and runtime-MCP wizard opens share the same mechanism (disposing clear() over a live reusable editor) but are not paths this PR's changelog claims: its scope is the selector/command-palette/queued-message overlay family named in #4604, whose open/restore contract is now uniformly detach-then-readd. Those sibling paths predate this PR on the base commit and are tracked as #4657 with per-site locations, default keybindings making them reachable, and the two-line fix shape — deliberately not scope-crept here to keep this PR one reviewable contract. Happy to fold them in if you'd rather have them in this PR.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

…y opens Issue #4657. Four overlay-open paths still cleared editorContainer with the live composer attached: the jobs overlay (alt+j //monitors), the tasks pane (alt+t), the OAuth API-key paste input, and the /debug log + raw-SSE viewers. Container.clear() disposes children terminally and Editor.dispose() tears down the tab-width change listener, so the first round-trip on any of these paths silently killed the composer's listener; every later restore re-mounted a dead editor and runtime tab-width changes stopped re-deriving composer layout. Each open now detaches the reusable editor first (Container.detachChild, the same detach-then-readd reuse contract showSelector and the extension-ui and pet-aware restore paths already follow from PR #4605), so the terminal clear disposes only the transient overlay. User-facing behavior is unchanged: the overlay mounts exactly as before and the composer is re-added on close. Regression coverage: one deterministic test per named path in composer-detach-overlay-paths.test.ts driving the real production open/close code (real JobsOverlayComponent/TasksPaneComponent focus lists, the captured production onPrompt closure over a real Input, and the real DebugSelector selection into the real DebugLogViewerComponent with an isolated agent-dir log source), each asserting the tab-width listener still fires across four overlay cycles, with a red control proving the probe detects genuine disposal. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes untouched (showSelector/extension-ui/pet restore shapes unchanged) Tested: red/green — 4/4 fail on unfixed tree, 4/4 pass with fix Tested: 107 pass across 9 adjacent suites (pet widget, editor dispose, tasks, jobs, debug, login) Tested: 190 pass across 7 selector/palette/input suites Tested: bun run --cwd=packages/coding-agent check + build clean; ci:test:smoke ok Tested: check:tools, node20-baseline, public-sync, schemas, docker-context, gjc-ui gates pass Confidence: high Scope-risk: narrow Reversibility: trivial

Yeachan-Heo · 2026-08-18T18:03:49Z

Fix-forward complete on head 6d12e769e6 — all substantive findings implemented; blocked solely on fresh human review

Head: 6d12e769e65f174a4615b87da9aea5d9b1b1318d · Base: dev ceb31349c2d024c75825c819cb530d5060f49380 · Digest: sha256:4221be9bfe1f9ffad63811decb0be31cc43369d59e3ad8b607522d28dee52eba · Branch: fix/issue-4604-command-palette-pet-restore (force-with-lease from eef82ef5dc → 11f772ac64 → 6d12e769e6 after two dev advances; contributor authorship Yeachan Heo <yeachan.heo@gmail.com> preserved on all four commits)
Every finding of reviews 4958123531 and 4958165282 is implemented (signed per-finding replies: issuecomment-5330457089):
1. Queued-message selector close/delete/move now restore through the pet-aware ctx.restoreComposer() — active pet keeps PetFramedEditor + composer reserve; 4 deterministic regressions added (65/65 in that suite).
2. Repository-wide SelectorController fixture audit: 9 additional test files beyond the flagged four now implement child-list-correct detachChild; real-Container and non-showSelector contexts verified unaffected.
3. Real CustomEditor fixtures disposed in all three pet-widget lifecycle tests — no process-wide tab-width listener leaks.
4. PR body evidence refreshed to the exact head/base/digest; superseded runs labeled truthfully (cancelled/failed-by-design), current run cited.
5. Adjacent raw clear() paths (OAuth code input, jobs/tasks panes, command-controller loaders, runtime-MCP wizard) audited: same mechanism, outside this PR's claimed contract, tracked in fix(tui): detach reusable composer before clear() at remaining overlay open paths (jobs, tasks, oauth paste, /debug) #4657 with per-site fix shape.
Exact-head CI: Dev CI 32154701523 (rerun) — every product job green, including both suites whose first attempt hit a runner-side Install system deps stall (recovered via cancel + rerun-failed; not a code failure — both pass locally and in the same workflow's other shards). Sole red is PR contract bootstrap failing by design on the recorded needs-human verdict.
Local exact-head validation: all 15+ named suites green, check clean, build exit 0, git diff --check clean.

Remaining blocker (human-only): a fresh non-author exact-head review of 6d12e769e6 with a merge-approved verdict bound to sha256:4221be9bfe1f9ffad63811decb0be31cc43369d59e3ad8b607522d28dee52eba. @snowykr — both your reviews are addressed head-on; re-review whenever ready. On approval the owner lane will flip the verdict to merge-approved, let the contract job go green, merge to dev, dogfood fresh dev with bun run build, and reconcile post-merge CI.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

snowykr

Verdict

Request changes. The reviewed commit is 6d12e769e65f174a4615b87da9aea5d9b1b1318d. Two evidence-grounded P1 issues remain: one leaves equivalent composer lifecycle defects in production paths, and one leaves the changed queued-message path without a regression test that exercises the relevant disposal contract.

Summary

The detach-before-clear repair is correctly applied in the reviewed selector, queued-message, and pet-remount paths. However, other production flows still clear a container with the reusable composer attached, and the queued-message tests use doubles that cannot detect the Container.clear() disposal behavior being fixed.

Findings / Required Changes

[P1] Repair the remaining composer replacement paths before clearing. packages/coding-agent/src/modes/controllers/command-controller.ts:191-199 still clears editorContainer while the reusable editor is attached in /share, then re-adds that disposed instance; the default Gist branch repeats the pattern at lines 248-256. The same lifecycle defect is reported in the MCP-add wizard, OAuth prompt, Jobs overlay, and Tasks pane. Because Container.clear() disposes attached children, these paths can remove the editor's tab-width listener and clear its paste state. Detach the reusable editor (or its framed owner) before clearing in each affected transition, and add focused regression coverage.
[P1] Cover the queued-message selector with real disposal semantics. packages/coding-agent/test/input-controller-keybindings.test.ts:198-207 uses a fake editor and a clear() mock that only truncates an array. Those tests would still pass if the new detachChild(editor) immediately before InputController's clear() were removed, so they do not prove that the changed path avoids Container.clear() → Editor.dispose(). Add an InputController-level regression using a real Container and CustomEditor; open and restore the queued-message selector, then verify the restored composer remains usable and responds to a tab-width change.

CI / Verification

No PR code, tests, linting, formatting, or local verification commands were run by the reviewers. GitHub-observed CI on the reviewed head showed the requested changed-test checks succeeding, including command-palette controller, resume-model controller, session-delete controller, command-palette interactive host, input-controller keybindings, and the relevant pet-widget suite; the overall Dev CI run was marked failed because PR contract bootstrap failed while its product checks succeeded.

Axis Coverage

A1 — Intent/architecture: Approved the scoped detach-before-clear lifecycle repair and changelog placement; no finding.
A2 — Correctness: Approved the changed selector, queued-message restoration, pet ownership, and error-path behavior; no finding.
A3 — Security: Found no actionable security, privacy, or trust issue.
A4 — Verification: Raised the queued-message real-disposal regression coverage gap.
A5 — Compatibility/completeness: Raised the remaining equivalent production composer-replacement lifecycle defects.

Yeachan-Heo · 2026-08-18T20:18:01Z

Base containment fixed — new exact head 6e5670266b on dev 648d13d72d; fresh review requested

Head: 6e5670266bf3901b513e865c4f991db765df417e · Base: dev 648d13d72d0749c068eb9278e04f2a6ba0b07b78 · Digest: sha256:4843cd465be7cab8657f34f03098f00a75c6af4252d4e4a2748b64b4f953eccf · Branch: fix/issue-4604-command-palette-pet-restore (force-with-lease from d9af0029f5; contributor authorship preserved on all four commits)
Cause of the CI cascade: dev advanced (08bef6cf88 → 648d13d72d, the em-dash display-only fix) after the previous push, so the recorded base became a non-ancestor. The branch is now rebased onto the live base — git merge-base is exactly 648d13d72d, containment satisfied.
Source delta is byte-identical to the reviewed scope across every rebase (same digest as d9af0029f5): the four PR commits unchanged, all five review-fix work streams intact (pet-aware queued-message restore + 4 regressions; repository-wide fixture detachChild audit across 9 files; real-editor fixture disposal; truthful evidence section; fix(tui): detach reusable composer before clear() at remaining overlay open paths (jobs, tasks, oauth paste, /debug) #4657 follow-up for adjacent paths).
Local exact-head validation on 6e5670266b: 263 focused tests / 0 fail across all 15 named suites; bun run --cwd=packages/coding-agent check clean; git diff --check clean.
Exact-head CI: Dev CI 32181054337 is running with base containment satisfied. The PR-contract digest-stale error is resolved by the body update above (verdict line now carries sha256:4843cd46... bound to this exact base...head).

@snowykr — your two reviews (4958123531, 4958165282) are fully addressed; the head has only been rebased since, with an unchanged source delta. Fresh exact-head review of 6e5670266b vs 648d13d72d, please — on approval the owner lane flips the verdict to merge-approved, lets the contract go green, merges to dev, dogfoods, and reconciles post-merge CI.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

…w-SSE Review follow-up on PR #4687 (snowykr CHANGES_REQUESTED): P1 — the branch assumed the generic SelectorController.showSelector open detach had already landed (PR #4605 is still unmerged), so production /debug and bare /login still disposed the reusable composer at that boundary before any downstream detach could run. showSelector now detaches the editor before its clear(), making this branch self-contained at every open path it claims. P2 — the debug regression only exercised the log viewer branch; it now alternates logs and raw-SSE entries across the four cycles, so both changed viewer branches carry the composer-lifecycle assertion. The harness also drives the real showDebugSelector/showSelector boundary instead of a local scaffold, so the P1 fix is covered by the same test (verified: reverting only the showSelector detach makes exactly this test fail). Lightweight test doubles whose editorContainer stubs predate detachChild (model-selector batch/profiles/profiles-redteam, provider-onboarding wizard and redteam, custom-model-preset-creation, login-preset-recommendation) gain the method, matching how PR #4605 updated its own doubles. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes (no other showSelector-site changes) Tested: stash of only the showSelector detach -> debug test fails (red, 3/4) Tested: 477 pass across 28 affected suites incl. the 4-path regression file Tested: bun run --cwd=packages/coding-agent check clean Confidence: high Scope-risk: narrow Reversibility: trivial

Yeachan-Heo · 2026-08-18T20:32:27Z

Exact-head CI green on 6e5670266b — sole remaining gate is the fresh non-author review

Head: 6e5670266bf3901b513e865c4f991db765df417e · Base: dev 648d13d72d0749c068eb9278e04f2a6ba0b07b78 (containment verified: merge-base is exactly the recorded base) · Digest: sha256:4843cd465be7cab8657f34f03098f00a75c6af4252d4e4a2748b64b4f953eccf
Dev CI 32181321301 on this exact head: every product job green — ts-build, cli-smoke, native-build, all 15 affected test shards (including input-controller-keybindings with the four new pet-aware queued-selector regressions, gajae-pet-widget, command-palette-interactive-host, selector-controller-command-palette, selector-controller-session-delete/-resume-model, theme-selector-input, thinking-selector, provider wizard/redteam, model-selector suites, login-preset-recommendation, custom-model-preset-creation), plan + evidence producer, Virtual integration validation, and gjc-state-gates. The single red job is PR contract bootstrap, failing by design on the recorded needs-human verdict — it flips green the moment a fresh non-author merge-approved verdict lands (no further push required).
The cancelled 32181054337 run was superseded mid-flight by the body-digest correction (its contract twin was reading the stale digest); its replacement is the green 32181321301 above. No product failure occurred at any point on this head.

@snowykr — every finding from your reviews 4958123531 and 4958165282 is implemented and verified on this exact head (pet-aware queued-message restore, repository-wide fixture detachChild audit, real-editor fixture disposal, truthful evidence, #4657 follow-up). The head has only been rebased since your review; the source delta is unchanged. A fresh exact-head review + merge-approved verdict is the last gate before dev merge.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

…y opens Issue #4657. Four overlay-open paths still cleared editorContainer with the live composer attached: the jobs overlay (alt+j //monitors), the tasks pane (alt+t), the OAuth API-key paste input, and the /debug log + raw-SSE viewers. Container.clear() disposes children terminally and Editor.dispose() tears down the tab-width change listener, so the first round-trip on any of these paths silently killed the composer's listener; every later restore re-mounted a dead editor and runtime tab-width changes stopped re-deriving composer layout. Each open now detaches the reusable editor first (Container.detachChild, the same detach-then-readd reuse contract showSelector and the extension-ui and pet-aware restore paths already follow from PR #4605), so the terminal clear disposes only the transient overlay. User-facing behavior is unchanged: the overlay mounts exactly as before and the composer is re-added on close. Regression coverage: one deterministic test per named path in composer-detach-overlay-paths.test.ts driving the real production open/close code (real JobsOverlayComponent/TasksPaneComponent focus lists, the captured production onPrompt closure over a real Input, and the real DebugSelector selection into the real DebugLogViewerComponent with an isolated agent-dir log source), each asserting the tab-width listener still fires across four overlay cycles, with a red control proving the probe detects genuine disposal. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes untouched (showSelector/extension-ui/pet restore shapes unchanged) Tested: red/green — 4/4 fail on unfixed tree, 4/4 pass with fix Tested: 107 pass across 9 adjacent suites (pet widget, editor dispose, tasks, jobs, debug, login) Tested: 190 pass across 7 selector/palette/input suites Tested: bun run --cwd=packages/coding-agent check + build clean; ci:test:smoke ok Tested: check:tools, node20-baseline, public-sync, schemas, docker-context, gjc-ui gates pass Confidence: high Scope-risk: narrow Reversibility: trivial

…w-SSE Review follow-up on PR #4687 (snowykr CHANGES_REQUESTED): P1 — the branch assumed the generic SelectorController.showSelector open detach had already landed (PR #4605 is still unmerged), so production /debug and bare /login still disposed the reusable composer at that boundary before any downstream detach could run. showSelector now detaches the editor before its clear(), making this branch self-contained at every open path it claims. P2 — the debug regression only exercised the log viewer branch; it now alternates logs and raw-SSE entries across the four cycles, so both changed viewer branches carry the composer-lifecycle assertion. The harness also drives the real showDebugSelector/showSelector boundary instead of a local scaffold, so the P1 fix is covered by the same test (verified: reverting only the showSelector detach makes exactly this test fail). Lightweight test doubles whose editorContainer stubs predate detachChild (model-selector batch/profiles/profiles-redteam, provider-onboarding wizard and redteam, custom-model-preset-creation, login-preset-recommendation) gain the method, matching how PR #4605 updated its own doubles. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes (no other showSelector-site changes) Tested: stash of only the showSelector detach -> debug test fails (red, 3/4) Tested: 477 pass across 28 affected suites incl. the 4-path regression file Tested: bun run --cwd=packages/coding-agent check clean Confidence: high Scope-risk: narrow Reversibility: trivial

… paths Review 4964398363 on the exact head found two P1 gaps: 1. Equivalent composer-replacement defects remained in production paths: /share custom-export and default-gist loaders, the OAuth API-key code input, the jobs overlay, the tasks pane, the MCP-add wizard, and the /debug log/raw-SSE viewers all cleared editorContainer with the live composer attached, so Container.clear() disposed the editor and its tab-width listener. Each open path now detaches the reusable composer first; close-path clears were already safe once their opens detach. This closes the follow-up scope filed in #4657. 2. The queued-message regressions used an array-double editorContainer, which cannot detect Container.clear() disposal. Added an InputController-level regression over a real Container + CustomEditor: three open/restore cycles through the queue selector with tab-width toggles proving the restored composer keeps its listener and input state (red-control verified: removing the open-path detach fails it). Also gave the /share temp-security double child-list-correct detachChild semantics. Lore-id: pr4605-review-4964398363 Tested: bun test input-controller-keybindings (66 incl. new real-disposal regression), share-command-temp-security, debug/log-viewer, and the full 15-suite focused surface (264 pass) Tested: bun run --cwd=packages/coding-agent check Confidence: high Scope-risk: moderate Reversibility: trivial

Yeachan-Heo · 2026-08-18T20:56:40Z

Both P1 findings of review 4964398363 implemented — new head 3196de229c, fresh review requested

Head: 3196de229c3980ed8886458f52c52147cf60b991 · Base: dev 648d13d72d0749c068eb9278e04f2a6ba0b07b78 · Digest: sha256:d7376d7fd523a31784246c3f91a23ad56e6b2843752e746bd723acce66857633

[P1] Remaining composer replacement paths — all repaired in 3196de229c with the same detach-before-clear contract, closing the #4657 follow-up scope:

Path	Site
`/share` custom-export loader	`command-controller.ts` `#shareExport` (both branches detach before their loader clear)
Default gist loader	`command-controller.ts` second `BorderedLoader` mount
OAuth API-key code input	`selector-controller.ts` `onPrompt` open (submit path was already self-healing)
Jobs overlay (`alt+j`, `/monitors`)	`selector-controller.ts` `showJobsOverlay` open
Tasks pane (`alt+t`)	`selector-controller.ts` `showTasksPane` open
MCP-add wizard	`runtime-mcp-command-controller.ts` wizard mount
`/debug` log + raw-SSE viewers	`debug/index.ts` both viewer mounts

A repository-wide sweep confirms every remaining editorContainer.clear() is either a close path (safe once its open detaches) or the deliberate editor-replacement teardown in InteractiveMode.replaceEditor (explicit previousEditor.dispose()). CHANGELOG updated to state the now-complete contract scope truthfully.

[P1] Real disposal semantics for the queued-message selector — added keeps a real composer alive across queued-message selector open/restore with real disposal semantics: a real Container + CustomEditor through InputController.handleDequeue(), three open/⎋-restore cycles, tab-width toggles per cycle proving the composer's listener survives (toBeGreaterThan(3)), plus final input-state assertion. Red-control verified locally: removing the open-path detachChild makes exactly this test fail (Expected: > 3); restoring it passes (66/66 in the suite). The /share temp-security double also gained child-list-correct detachChild semantics.

Validation on 3196de229c: 264 focused tests / 0 fail across all 15 named suites + share-command-temp-security, debug/log-viewer, mcp-quarantine-surface, credential-auto-import-flows, interactive-mode-editor-component, resume-session-reentrancy; check clean; git diff --check clean. Exact-head CI is starting on this head.

@snowykr — both P1s of your latest review are addressed head-on in 3196de229c. Fresh exact-head review when ready; on approval the verdict flips to merge-approved and the PR merges to dev.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

Yeachan-Heo · 2026-08-18T21:15:14Z

Exact-head CI green on 3196de229c — all P1 fixes verified; fresh non-author review is the last gate

Head: 3196de229c3980ed8886458f52c52147cf60b991 · Base: dev 648d13d72d0749c068eb9278e04f2a6ba0b07b78 · Digest: sha256:d7376d7fd523a31784246c3f91a23ad56e6b2843752e746bd723acce66857633
Dev CI 32185025067 on this exact head: every product job green — all affected test shards (including the new share-command-temp-security shard from the /share fix and the expanded input-controller-keybindings suite carrying the real-Container/CustomEditor disposal regression, red-control verified locally), gajae-pet-widget, command-palette-interactive-host, every selector suite, ts-build, cli-smoke, native-build, plan + evidence producer, Virtual integration validation, gjc-state-gates. The single red is PR contract bootstrap, failing by design on the recorded needs-human verdict — it flips green automatically once a fresh non-author merge-approved verdict is recorded (no push needed).
Both P1 findings of review 4964398363 are implemented in this head (see issuecomment-5333962892): the detach-before-clear contract now covers every overlay open path that cleared the shared container with the live composer attached (/share both branches, OAuth code input, jobs overlay, tasks pane, MCP-add wizard, /debug viewers), and the queued-message selector has a real-disposal regression that fails when the open-path detach is removed.

@snowykr — fresh exact-head review of 3196de229c vs 648d13d72d, please. On approval: verdict flips to merge-approved bound to this digest, contract goes green, squash-merge to dev, dogfood bun run build on fresh dev, post-merge CI reconciled.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

snowykr

Verdict

CHANGES_REQUESTED — reviewed exact head 3196de229c3980ed8886458f52c52147cf60b991 against dev base 648d13d72d0749c068eb9278e04f2a6ba0b07b78.

Summary

The raw-editor detach-before-clear() direction addresses the stated terminal-disposal defect. However, pet-enabled overlay flows still detach and restore the raw editor while the mounted child is PetFramedEditor. That leaves the actual child attached when Container.clear() runs, so it is disposed and later reused. The regression coverage also does not exercise this controller-level pet path.

Findings / Required Changes

[P1] Detach and restore the composition that is actually mounted in pet mode.
input-controller.ts:1358-1360 detaches only ctx.editor, even though input-controller.ts:1283-1285 establishes that an active pet mounts PetFramedEditor. The detach is therefore a no-op in pet mode; the following terminal clear() disposes the framed composer, and the close path reattaches a disposed child. The same raw-editor-only pattern remains in the changed selector, command, and MCP overlay paths (for example selector-controller.ts:1344-1346, command-controller.ts:193-195, and runtime-mcp-command-controller.ts:508-510). Route these transitions through one pet-aware detach/restore authority so the mounted composition—not just its inner editor—is preserved across every clear().
[P1] Add a controller-level regression for the active-pet queued-message selector lifecycle.
The current pet-widget test clears the frame and checks object identity (gajae-pet-widget.test.ts:686-703), while its repeated real-editor disposal probe is never activated (gajae-pet-widget.test.ts:710-763). It cannot detect the queued-message controller disposing the real mounted child. Exercise active-pet queue-selector cancel, final-item delete, and move/refresh transitions, then assert that input remains usable and tab-width invalidation still reaches the composer after restoration.
[P2] Preserve focus and rendering when replacing the custom-share overlay.
command-controller.ts:193-195 removes the loader without the ui.setFocus(loader) and ui.requestRender() transition setup. Restore those calls (or provide the equivalent through the shared composition-aware helper) so the custom-share loader is immediately focused and rendered after the swap.

CI / Verification

Reviewed GitHub Actions evidence for this exact head; the scoped selector suites, gajae-pet-widget.test.ts, input-controller-keybindings.test.ts, and check:@gajae-code/coding-agent are reported as passing in Dev CI #32186689415.
The remaining exact-head failures are the needs-human governance checks (PR contract bootstrap / Validate exact-head PR contract), which are excluded from this verdict.
This review was static/CI-artifact based; no PR code, test suite, build, linter, or formatter was executed.

Axis Coverage

Axis	Result
A1. Intent / Policy / Contract	Block — the supported pet-mode contract is not preserved.
A2. Architecture / Correctness / Failure	Block — `clear()` can still dispose the mounted pet-framed composer.
A3. Security / Privacy / Trust	No actionable finding — no changed trust-boundary regression identified.
A4. Verification / Tests / CI	Block — the pet-mode controller disposal path lacks a detecting regression.
A5. Context / Compatibility / Platform	Block — restored active-pet overlays can lose their framed composer behavior.

…y opens Issue #4657. Four overlay-open paths still cleared editorContainer with the live composer attached: the jobs overlay (alt+j //monitors), the tasks pane (alt+t), the OAuth API-key paste input, and the /debug log + raw-SSE viewers. Container.clear() disposes children terminally and Editor.dispose() tears down the tab-width change listener, so the first round-trip on any of these paths silently killed the composer's listener; every later restore re-mounted a dead editor and runtime tab-width changes stopped re-deriving composer layout. Each open now detaches the reusable editor first (Container.detachChild, the same detach-then-readd reuse contract showSelector and the extension-ui and pet-aware restore paths already follow from PR #4605), so the terminal clear disposes only the transient overlay. User-facing behavior is unchanged: the overlay mounts exactly as before and the composer is re-added on close. Regression coverage: one deterministic test per named path in composer-detach-overlay-paths.test.ts driving the real production open/close code (real JobsOverlayComponent/TasksPaneComponent focus lists, the captured production onPrompt closure over a real Input, and the real DebugSelector selection into the real DebugLogViewerComponent with an isolated agent-dir log source), each asserting the tab-width listener still fires across four overlay cycles, with a red control proving the probe detects genuine disposal. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes untouched (showSelector/extension-ui/pet restore shapes unchanged) Tested: red/green — 4/4 fail on unfixed tree, 4/4 pass with fix Tested: 107 pass across 9 adjacent suites (pet widget, editor dispose, tasks, jobs, debug, login) Tested: 190 pass across 7 selector/palette/input suites Tested: bun run --cwd=packages/coding-agent check + build clean; ci:test:smoke ok Tested: check:tools, node20-baseline, public-sync, schemas, docker-context, gjc-ui gates pass Confidence: high Scope-risk: narrow Reversibility: trivial

…w-SSE Review follow-up on PR #4687 (snowykr CHANGES_REQUESTED): P1 — the branch assumed the generic SelectorController.showSelector open detach had already landed (PR #4605 is still unmerged), so production /debug and bare /login still disposed the reusable composer at that boundary before any downstream detach could run. showSelector now detaches the editor before its clear(), making this branch self-contained at every open path it claims. P2 — the debug regression only exercised the log viewer branch; it now alternates logs and raw-SSE entries across the four cycles, so both changed viewer branches carry the composer-lifecycle assertion. The harness also drives the real showDebugSelector/showSelector boundary instead of a local scaffold, so the P1 fix is covered by the same test (verified: reverting only the showSelector detach makes exactly this test fail). Lightweight test doubles whose editorContainer stubs predate detachChild (model-selector batch/profiles/profiles-redteam, provider-onboarding wizard and redteam, custom-model-preset-creation, login-preset-recommendation) gain the method, matching how PR #4605 updated its own doubles. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes (no other showSelector-site changes) Tested: stash of only the showSelector detach -> debug test fails (red, 3/4) Tested: 477 pass across 28 affected suites incl. the 4-path regression file Tested: bun run --cwd=packages/coding-agent check clean Confidence: high Scope-risk: narrow Reversibility: trivial

…y opens Issue #4657. Four overlay-open paths still cleared editorContainer with the live composer attached: the jobs overlay (alt+j //monitors), the tasks pane (alt+t), the OAuth API-key paste input, and the /debug log + raw-SSE viewers. Container.clear() disposes children terminally and Editor.dispose() tears down the tab-width change listener, so the first round-trip on any of these paths silently killed the composer's listener; every later restore re-mounted a dead editor and runtime tab-width changes stopped re-deriving composer layout. Each open now detaches the reusable editor first (Container.detachChild, the same detach-then-readd reuse contract showSelector and the extension-ui and pet-aware restore paths already follow from PR #4605), so the terminal clear disposes only the transient overlay. User-facing behavior is unchanged: the overlay mounts exactly as before and the composer is re-added on close. Regression coverage: one deterministic test per named path in composer-detach-overlay-paths.test.ts driving the real production open/close code (real JobsOverlayComponent/TasksPaneComponent focus lists, the captured production onPrompt closure over a real Input, and the real DebugSelector selection into the real DebugLogViewerComponent with an isolated agent-dir log source), each asserting the tab-width listener still fires across four overlay cycles, with a red control proving the probe detects genuine disposal. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes untouched (showSelector/extension-ui/pet restore shapes unchanged) Tested: red/green — 4/4 fail on unfixed tree, 4/4 pass with fix Tested: 107 pass across 9 adjacent suites (pet widget, editor dispose, tasks, jobs, debug, login) Tested: 190 pass across 7 selector/palette/input suites Tested: bun run --cwd=packages/coding-agent check + build clean; ci:test:smoke ok Tested: check:tools, node20-baseline, public-sync, schemas, docker-context, gjc-ui gates pass Confidence: high Scope-risk: narrow Reversibility: trivial

…w-SSE Review follow-up on PR #4687 (snowykr CHANGES_REQUESTED): P1 — the branch assumed the generic SelectorController.showSelector open detach had already landed (PR #4605 is still unmerged), so production /debug and bare /login still disposed the reusable composer at that boundary before any downstream detach could run. showSelector now detaches the editor before its clear(), making this branch self-contained at every open path it claims. P2 — the debug regression only exercised the log viewer branch; it now alternates logs and raw-SSE entries across the four cycles, so both changed viewer branches carry the composer-lifecycle assertion. The harness also drives the real showDebugSelector/showSelector boundary instead of a local scaffold, so the P1 fix is covered by the same test (verified: reverting only the showSelector detach makes exactly this test fail). Lightweight test doubles whose editorContainer stubs predate detachChild (model-selector batch/profiles/profiles-redteam, provider-onboarding wizard and redteam, custom-model-preset-creation, login-preset-recommendation) gain the method, matching how PR #4605 updated its own doubles. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes (no other showSelector-site changes) Tested: stash of only the showSelector detach -> debug test fails (red, 3/4) Tested: 477 pass across 28 affected suites incl. the 4-path regression file Tested: bun run --cwd=packages/coding-agent check clean Confidence: high Scope-risk: narrow Reversibility: trivial

…y opens Issue #4657. Four overlay-open paths still cleared editorContainer with the live composer attached: the jobs overlay (alt+j //monitors), the tasks pane (alt+t), the OAuth API-key paste input, and the /debug log + raw-SSE viewers. Container.clear() disposes children terminally and Editor.dispose() tears down the tab-width change listener, so the first round-trip on any of these paths silently killed the composer's listener; every later restore re-mounted a dead editor and runtime tab-width changes stopped re-deriving composer layout. Each open now detaches the reusable editor first (Container.detachChild, the same detach-then-readd reuse contract showSelector and the extension-ui and pet-aware restore paths already follow from PR #4605), so the terminal clear disposes only the transient overlay. User-facing behavior is unchanged: the overlay mounts exactly as before and the composer is re-added on close. Regression coverage: one deterministic test per named path in composer-detach-overlay-paths.test.ts driving the real production open/close code (real JobsOverlayComponent/TasksPaneComponent focus lists, the captured production onPrompt closure over a real Input, and the real DebugSelector selection into the real DebugLogViewerComponent with an isolated agent-dir log source), each asserting the tab-width listener still fires across four overlay cycles, with a red control proving the probe detects genuine disposal. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes untouched (showSelector/extension-ui/pet restore shapes unchanged) Tested: red/green — 4/4 fail on unfixed tree, 4/4 pass with fix Tested: 107 pass across 9 adjacent suites (pet widget, editor dispose, tasks, jobs, debug, login) Tested: 190 pass across 7 selector/palette/input suites Tested: bun run --cwd=packages/coding-agent check + build clean; ci:test:smoke ok Tested: check:tools, node20-baseline, public-sync, schemas, docker-context, gjc-ui gates pass Confidence: high Scope-risk: narrow Reversibility: trivial

…w-SSE Review follow-up on PR #4687 (snowykr CHANGES_REQUESTED): P1 — the branch assumed the generic SelectorController.showSelector open detach had already landed (PR #4605 is still unmerged), so production /debug and bare /login still disposed the reusable composer at that boundary before any downstream detach could run. showSelector now detaches the editor before its clear(), making this branch self-contained at every open path it claims. P2 — the debug regression only exercised the log viewer branch; it now alternates logs and raw-SSE entries across the four cycles, so both changed viewer branches carry the composer-lifecycle assertion. The harness also drives the real showDebugSelector/showSelector boundary instead of a local scaffold, so the P1 fix is covered by the same test (verified: reverting only the showSelector detach makes exactly this test fail). Lightweight test doubles whose editorContainer stubs predate detachChild (model-selector batch/profiles/profiles-redteam, provider-onboarding wizard and redteam, custom-model-preset-creation, login-preset-recommendation) gain the method, matching how PR #4605 updated its own doubles. Lore-id: issue-4657-composer-detach Constraint: preserve PR #4605 lanes (no other showSelector-site changes) Tested: stash of only the showSelector detach -> debug test fails (red, 3/4) Tested: 477 pass across 28 affected suites incl. the 4-path regression file Tested: bun run --cwd=packages/coding-agent check clean Confidence: high Scope-risk: narrow Reversibility: trivial

Yeachan-Heo mentioned this pull request Aug 16, 2026

fix(dev): command-palette-interactive-host failures from #4591 pet integration #4604

Closed

Yeachan-Heo force-pushed the fix/issue-4604-command-palette-pet-restore branch from c194e79 to b5929c8 Compare August 17, 2026 16:01

Yeachan-Heo requested a review from snowykr August 17, 2026 16:11

snowykr requested changes Aug 17, 2026

View reviewed changes

Yeachan-Heo requested a review from snowykr August 17, 2026 19:15

Yeachan-Heo force-pushed the fix/issue-4604-command-palette-pet-restore branch from a890974 to cce1441 Compare August 17, 2026 19:23

Yeachan-Heo force-pushed the fix/issue-4604-command-palette-pet-restore branch from 097c3b4 to eef82ef Compare August 18, 2026 04:37

Yeachan-Heo mentioned this pull request Aug 18, 2026

fix(tui): detach reusable composer before clear() at remaining overlay open paths (jobs, tasks, oauth paste, /debug) #4657

Open

snowykr requested changes Aug 18, 2026

View reviewed changes

Yeachan-Heo force-pushed the fix/issue-4604-command-palette-pet-restore branch from eef82ef to 11f772a Compare August 18, 2026 15:13

Yeachan-Heo mentioned this pull request Aug 18, 2026

fix(tui): detach reusable composer before clear() at remaining overlay open paths #4687

Open

5 tasks

Yeachan-Heo force-pushed the fix/issue-4604-command-palette-pet-restore branch from 11f772a to 6d12e76 Compare August 18, 2026 15:22

snowykr requested changes Aug 18, 2026

View reviewed changes

Yeachan-Heo force-pushed the fix/issue-4604-command-palette-pet-restore branch 2 times, most recently from d9af002 to 6e56702 Compare August 18, 2026 20:13

snowykr requested changes Aug 18, 2026

View reviewed changes

Conversation

Yeachan-Heo commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Current-dev value and non-duplication

Reconstruction (base moved dev 6696988 → 27afb73 → 2bd7b4a → ceb3134 → 08bef6c → 648d13d)

Exact evidence

Review state

GJC verdict

Uh oh!

chatgpt-codex-connector Bot commented Aug 16, 2026

Uh oh!

Yeachan-Heo commented Aug 16, 2026

Root cause

Validation (exact PR head c194e79)

CI on this head (run 31932247745 + 31932247752)

Uh oh!

Yeachan-Heo commented Aug 17, 2026

Uh oh!

Yeachan-Heo commented Aug 17, 2026

Uh oh!

snowykr left a comment

Choose a reason for hiding this comment

Verdict: Request changes

Required changes

1. Make the remount tests model the reusable editor lifecycle

2. Refresh the exact-head evidence in the PR contract

Review lane summary

Uh oh!

Yeachan-Heo commented Aug 17, 2026

Uh oh!

Yeachan-Heo commented Aug 17, 2026

Uh oh!

Yeachan-Heo commented Aug 17, 2026

Uh oh!

Yeachan-Heo commented Aug 17, 2026

Uh oh!

Yeachan-Heo commented Aug 17, 2026

Uh oh!

Yeachan-Heo commented Aug 18, 2026

Uh oh!

Yeachan-Heo commented Aug 18, 2026

Uh oh!

snowykr left a comment

Choose a reason for hiding this comment

Verdict: Request changes

Required change

Update all SelectorController test contexts for the new detachChild requirement

Verification notes

Uh oh!

snowykr left a comment

Choose a reason for hiding this comment

Verdict: Request changes

Required changes

1. Restore the pet-aware composer from the queued-message selector

2. Update every selector test double that now needs detachChild

3. Refresh the exact-head evidence references

Additional verification note

Lane summary

Uh oh!

Yeachan-Heo commented Aug 18, 2026

Uh oh!

Yeachan-Heo commented Aug 18, 2026

Uh oh!

snowykr left a comment

Choose a reason for hiding this comment

Verdict

Summary

Findings / Required Changes

CI / Verification

Axis Coverage

Uh oh!

Yeachan-Heo commented Aug 18, 2026

Uh oh!

Yeachan-Heo commented Aug 18, 2026

Uh oh!

Yeachan-Heo commented Aug 18, 2026

Uh oh!

Yeachan-Heo commented Aug 18, 2026

Uh oh!

Yeachan-Heo commented Aug 16, 2026 •

edited

Loading

Reconstruction (base moved dev `6696988` → `27afb73` → `2bd7b4a` → `ceb3134` → `08bef6c` → `648d13d`)

Validation (exact PR head `c194e79`)

Update all SelectorController test contexts for the new `detachChild` requirement

2. Update every selector test double that now needs `detachChild`